Detect usage of block-scoped variables from earlier case blocks#47160
Detect usage of block-scoped variables from earlier case blocks#47160RyanCavanaugh wants to merge 5 commits intomicrosoft:mainfrom
Conversation
DanielRosenwasser
left a comment
DanielRosenwasser
left a comment
There was a problem hiding this comment.
Looks good, just thinking about ways we can give some more context to the error when possible.
| const usageCaseBlock = getAncestor(node, SyntaxKind.CaseClause); | ||
| if (usageCaseBlock) { | ||
| if (usageCaseBlock.pos > symbol.valueDeclaration.pos) { | ||
| error(node, Diagnostics.Variable_0_is_declared_in_a_prior_case_block, symbolToString(symbol)); |
There was a problem hiding this comment.
Add a "'0' is declared here" related span to the valueDeclaration.
| "category": "Error", | ||
| "code": 2836 | ||
| }, | ||
| "Variable '{0}' is declared in a prior case block.": { |
There was a problem hiding this comment.
case needs to be quoted
| "Variable '{0}' is declared in a prior case block.": { | |
| "Variable '{0}' is declared in a prior 'case' block.": { | |
It's tough to give some more context, but I think it could be helpful. Maybe something like
| "Variable '{0}' is declared in a prior case block.": { | |
| "Variable '{0}' is declared in a prior 'case' block and cannot be referenced if that declaration does not execute.": { |
|
@typescript-bot user test this |
|
Heya @RyanCavanaugh, I've started to run the parallelized community code test suite on this PR at e223f1b. You can monitor the build here. |
|
The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master. |
|
@typescript-bot test this |
|
Heya @RyanCavanaugh, I've started to run the extended test suite on this PR at e223f1b. You can monitor the build here. |
|
FWIW, despite technically being a false positive, I find the sample code in the OP to be... somewhat suspicious, to put it lightly. Fallthrough is a thing, but |
|
@RyanCavanaugh do you want to come back to this? It's been sitting for a long time. |
|
I'm not super keen on this one to be honest. |
5 participants
Fixes #19503
Note that this has the possibility to incur a false positive: